fix(comments): rebuild the comment composer on the create-post UX - #6417
fix(comments): rebuild the comment composer on the create-post UX#6417tsahimatsliah wants to merge 35 commits into
Conversation
The reported bug: on mobile a long comment overflowed with nowhere to scroll, so the text ended up hidden behind the virtual keyboard. Root cause was two-fold. `CommentMarkdownInput` drove `RichTextInput` with the default top toolbar, which renders no scroll container and no max-height, so the editor just grew. `CommentModal` then force-sized the form to the visual viewport by writing `style.height` during render — producing the literal "autopx" whenever the maths went negative. The composer is now the same inline box as the watercooler create-post composer, minus title, cover, post type and audience: bottom toolbar, one action bar, and a height capped against `visualViewport` (the part that survives the keyboard) with the body scrolling inside it. Same component on every viewport, so `CommentModal` and the mobile/desktop fork are gone. The mobile floating bar now opens the in-page composer through a window event instead of mounting its own copy inside the fixed footer. Several fixes landed in shared primitives and so reach every composer, including create post: - RichTextInput: rich and markdown modes share one tree, so switching no longer remounts the avatar (which refetched the image and blinked) or the action bar. Heights now match between modes, and the scroll offset and caret are preserved across the toggle. - RichTextToolbar: the overflow budget ignored dividers and row gaps, and the overflow button lived inside the `overflow-hidden` group it exists to protect, so it got sliced. Also swapped to the three-dots icon and kept it beside the formatting items. - Drawer: closing on any click whose target was outside the panel also caught portaled dropdowns, whose synthetic clicks bubble up the React tree. Picking a post type, audience, poll duration or schedule tore the drawer down instead of running the action. Now only a backdrop hit closes it. - Switch: the label could not shrink, so long copy ran off the right edge on mobile. - ProseMirror no longer adds a second min-height inside the editor's own padding. - AudienceChip and the poll option input could not shrink either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Swapping the rich editor for the textarea briefly shrinks the scroll container, so the browser clamps its offset to 0. Restoring from a later animation frame was too late in one direction: markdown to rich held, but rich to markdown always landed back at the top. Restore right after the textarea gets its real height, before paint, guarded by a flag so normal typing does not fight the user's own scrolling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Engineering review pass over the composer. No behaviour or visual change. - Stories move under stories/components/comments/, next to the existing CommentContainer story, and are retitled Components/Comments/*. The storybook AGENTS.md groups stories by atomic design, and a top-level `comments/` group also sorted outside preview.tsx's storySort order. Added the `autodocs` tag the same guide asks for. - The header label goes through Typography rather than a raw span with typo-* classes, per the components guide. - The composer form carries an aria-label, so it exposes role="form" to assistive tech. The spec queries that instead of a data-testid, which the components guide reserves for when no accessible query exists. - Drop three props left dead by removing CommentModal: `style`, `showUserAvatar` and `CommentClassName.markdownContainer`. Nothing passed them any more, and the comment composer always wants its avatar. - Flatten CommentInput's `className.input` nesting to `CommentClassName`. The wrapper object only existed to sit beside the `modal` key that went with the deleted modal branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The changed-file guard reports all strict errors in a touched file, not just newly introduced ones, so editing these files made their existing violations block CI. Fixed rather than suppressed; no behaviour change. - isSourcePublicSquad accepts an optional Source, which is what its body already assumes via optional chaining. - PostEngagements: useRef<NewCommentRef>(null) for a LegacyRef-compatible ref, and `false | value` unions replaced with undefined branches. - Drawer: container ref nullable, forwardRef render fn typed with ForwardedRef. The debounced close now forwards the event that triggered it instead of dropping it and calling onClose with undefined. - PostComments.modalParentSelector may return null, matching getCompanionWrapper; the call site already null-coalesced it. - CompanionDiscussion returns ReactElement | null, which it always did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mobile round of feedback on the composer surfaces: - On mobile the comment composer opens as a full-screen Drawer, the same way creating a post does — an inline box wedged into the page is too small to write in. It autofocuses so the keyboard opens ready to type. A new `fills` mode makes the composer fill the drawer instead of capping against the viewport, dropping the inline card treatment. - Full-screen drawers size themselves to the visual viewport (height and offsetTop), so bottom action bars sit above the virtual keyboard instead of underneath it. useVisualViewport now reports offsetTop and listens to viewport scroll for the iOS keyboard-pan case. - The create-post kind picker moves to its own row above the action bar on mobile (`stackToolbarLeading`). The bar's overflow menu only relocates formatting items, so the picker plus the icon actions overflowed the clipped group and rendered sliced. - Expand/collapse is hidden on mobile — the composer is already full-screen there, so the control was a no-op. - Drawer wrapper overrides use `!px-0 !pt-0` (matching NavDrawer): plain `p-0` lost to the drawer's default padding, so the composer paid padding twice, and killing padding-bottom outright would also have dropped the safe-area inset that `drawer-padding` provides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Animated/transformed ancestors (the animate-composer-in wrapper) create a containing block for position: fixed, pinning the drawer mid-page with the page showing through. appendOnRoot portals it to the document root so it actually covers the screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…er on mobile - The stacked leading row rendered in addition to the inline slot rather than instead of it, so mobile showed two kind pickers. The inline slot now yields when stackLeading is set. - On mobile the schedule calendar moves up beside the header's scheduled- posts clock, where scheduling already lives; the bottom bar keeps just the Post button. Desktop keeps both beside Post. Specs for this mobile round: toolbar leading-slot dedupe, the comment drawer (full-screen, portaled, autofocus, fills, padding override), keyboard-safe drawer sizing, backdrop-only close, header expand/schedule placement per breakpoint, and the composer fills mode. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…loating bar on redesign - Queue an early imperative focus until the TipTap editor exists, so reply composers autofocus on desktop (the mount-time focus raced editor creation) - Move the More-formatting tooltip onto the trigger's tooltip prop; the Tooltip wrapper blurred the trigger on mouseup, insta-dismissing the menu - Extract useOpenPostCommentRequest and register it in PostDiscussionPanel too, so the mobile floating bar opens the composer on the redesigned post page, not only the classic one - Cover the full PR surface with tests: useVisualViewport, postComment event bridge, Switch wrap fix, RichTextInput avatar/mode/focus, FooterWrapper dispatch, PostPage integration, toolbar overflow menu Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scrolling inside the mobile composer drawer chained to the post page behind it, which visibly jumped and shifted. Apply the same body lock react-modal uses (hidden-scrollbar, ref-counted for stacked drawers) and contain overscroll on the drawer's own scroller; the composer's internal scrolling is untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
<html> is the page's actual scroll container, so the body-level lock never reached the viewport and the page behind the drawer kept scrolling. Verified against the live preview. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The drawer's safe-area bottom padding stacked with the action bar's own pb-5, reading as double spacing under the actions. The bar now absorbs the safe area itself (max(1.25rem, safe-area-inset-bottom)) and the drawer adds none, so the composer sits 20px from every edge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tsahimatsliah
left a comment
There was a problem hiding this comment.
Final review
Third pass, against 265ebb3. Everything I raised last time is closed, and closed properly rather than papered over:
- Scroll lock is now scoped and honest — gated on
isFullScreen, previous inlineoverflowcaptured and restored, ref-counted for stacking, with tests for each branch. Context menus and pickers behave exactly as they did before. useVisualViewport(enabled)— partial drawers no longer subscribe toresize/scroll, so an open iOS keyboard doesn't re-render every mounted drawer subtree..ProseMirrorflexes instead ofheight: 100%— the right fix, and you verified theShareLinkcase on the preview rather than reasoning about it.min-h-fullwould have failed the same way, so the choice matters.- Description now matches the branch, and the outstanding-device-pass caveat is stated up front instead of listing shipped work as deferred.
- The
Laptopfork is answered: matchingSmartComposerModalso "writing a comment behaves like writing a post" holds on every non-laptop width.
I re-traced the event bridge over the whole surface this time, not just the two registrations: PostEngagements (classic) and PostDiscussionPanel (redesign, via PostFocusCard, rendered unconditionally) are the only surfaces FooterWrapper can sit under with a post, and EngagementRail drives its own ReaderFloatingActionBar rather than the footer bar — so there is no surface where the floating bar's tap lands on nothing. shouldHandleCommentQuery survived the FooterWrapper change because the page-level NewComment was only CSS-hidden on mobile, never unmounted. No dangling CommentInputOrModal / CommentModal references. CI green including typecheck_strict_changed.
Blocking
Only the one you already flagged: the real-device pass. Nothing in the code is blocking. But this branch now ships a full-screen drawer positioned from visualViewport.offsetTop, an <html>-level scroll lock, and env(safe-area-inset-bottom) padding — the three things emulation cannot honestly verify, and offsetTop in particular diverges on real iOS Safari. One pass on hardware (iOS Safari + Chrome, Android) and I'm happy to recommend merge.
Non-blocking
See the four inline notes. Two more that don't attach cleanly to a line:
Escape and focus trap left with CommentModal. react-modal gave the old composer Escape-to-close and a focus trap; Drawer has neither (no drawer in the repo does). Below Laptop includes an iPad with a keyboard, where the X is now the only way out. Consistent with existing drawer behaviour, so not a regression I'd hold the PR for — worth a follow-up if the composer becomes the main writing surface.
Two create-post deltas that read as intentional but aren't in the description. SmartComposerModal now renders notificationToggleNode in markdown mode (previously !isMarkdownMode), and the shared avatar block means CreateSharedPostModal shows its avatar in markdown mode too, at ml-4 mt-4 instead of ml-3 mt-3 with the editor's indent moving from ml-3 tablet:ml-0 to !pl-3. Both follow from unifying the two trees; a glance at that modal on mobile would confirm the spacing still reads right.
Verification
- Full three-dot diff re-read at
265ebb3, plus2dfc541..265ebb3in isolation - Every previous finding re-checked against the current code
- Event bridge traced across all four composer surfaces and both post-page variants
- Scroll-lock capture/restore and stacking semantics re-derived
- Searched for dangling references to the deleted modal and its props
- CI green,
typecheck_strict_changedincluded - Real-device pass — blocking item above
Verdict
Comment. The code is in good shape and I'd approve it on the strength of the review alone; what's left is hardware confirmation, which I can't do for you.
Reviewed by AI.
- CommentMarkdownInput passes enabled=!fills to useVisualViewport, so the mobile drawer stops re-rendering on iOS keyboard scroll for a value it discards - the toolbar overflow budget measures the rendered divider and the row's column-gap instead of hand-synced constants (jsdom keeps fallbacks) - the open comment composer owns its focus through the queued autofocus: the by-id helper races the lazy chunk and TipTap's async editor - a title-only freeform edit no longer demands a body to save Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Preserve the comment draft when crossing the Laptop breakpoint (tablet rotation) remounts the editor between drawer and inline trees. - Warn in dev when requestOpenPostComment fires with no registered listener, since the floating bar's tap otherwise fails silently; subscription now goes through subscribeOpenPostComment. - Back out the unrelated freeform-edit validation change in useComposerSubmit (belongs in its own PR) and the pure comment rewordings in useComposerSubmit/SmartComposerModal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
main's tools pages landed against the old CommentInputOrModal, which this branch deletes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mposer The deleted CommentModal carried the NotificationPromptSource.NewComment switch; it now stacks above the composer's bottom action bar and primes the prompt only after a successful submit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…text The floating bar and the composer share the page's provider, so the window CustomEvent bridge, its listener count, and the dev-only warning all go away; the PostPage test now drives the real button end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Below the Laptop breakpoint the full-screen drawer would scroll-lock the host page's <html> and cover the article being read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changes
Reported by a user: "the post comment input is broken on mobile… the input element or its wrapper is not scrollable and my long comments overflow. When this happens I can no longer see my text, because it's hidden behind the virtual keyboard."
Two causes.
CommentMarkdownInputdroveRichTextInputwith the default top toolbar, which renders no scroll container and no max-height — so the editor grew without bound.CommentModalthen force-sized the form to the visual viewport by writingstyle.heightduring render, producing the literal value"autopx"whenever the maths went negative.Rather than patch the modal, the composer was rebuilt on the create-post UX (the watercooler inline composer), minus title, cover, post type and audience.
Comment composer
visualViewport(the part that survives the keyboard), body scrolling inside it.CommentModalis deleted;CommentInputOrModal→CommentInput.Laptop:Tablet(656px); this one goes full-screen belowLaptop(1020px), so tablet users move from inline input to the drawer. Deliberate, matchingSmartComposerModal's own!isLaptopfork — the composer gets the whole screen and opens focused, so the keyboard comes up ready to type. Portalled withappendOnRoot, because animated ancestors (animate-composer-in) trapposition: fixedand pinned it mid-page.visualViewportheight andoffsetTop, so the action bar sits above the keyboard on iOS, which pans the layout viewport instead of resizing it.hidden-scrollbar+ inlineoverflowon<html>, ref-counted, previous value restored), and the drawer's own scroller usesoverscroll-contain. Scrolling the composer no longer drags the post page behind it.Replying to @user(comment author → post author → source), orEditing your comment. Markdown toggle + close on the right.useOpenPostCommentRequest) instead of mounting its own copy inside the fixed footer. BothPostEngagementsandPostDiscussionPanelregister, so the redesigned post page answers it too.hidden tablet:flex). Deliberate: it is the redesign's primary entry point under the post, and the floating bar's comment tap opens the same composer.Create post — same shared components, so these land there as well
Shared primitives
focus()requested before the editor exists is queued rather than dropped, which is what left reply composers unfocused.overflow-hiddengroup it exists to protect, so it was sliced. Now three-dots, beside the formatting items. Its tooltip rides onDropdownMenuTrigger's owntooltipprop; wrapping the trigger inTooltipblurred it on mouseup and could dismiss the menu before it painted.offsetTopandscrollfor the iOS keyboard case, and takes anenabledflag so only consumers that need it subscribe.height: 100%, which resolved toautowherever the box height comes fromminHeightClassNameand let the editable collapse away from clicks below the first line.Tests / stories
New specs:
CommentMarkdownInput(16),CommentInput,Drawer(viewport sizing, backdrop-only close, scroll lock, stacking, overflow restore),useVisualViewport,useOpenPostCommentRequest,RichTextToolbar,Switch,postComment,FooterWrapper, plus additions toRichTextInput,SmartComposerModalandPostPage.New stories under
stories/components/comments/—Components/Comments/Composer(13 cases) andComponents/Comments/Composer states(all-states gallery + mobile widths).Conventions / types
Typography; the composer form carries anaria-label, so it exposesrole="form"and the spec queries that instead of adata-testid.CommentModal:style,showUserAvatar,CommentClassName.markdownContainer, and theclassName.inputnesting that only existed beside the oldmodalkey.PostEngagements,Drawer,CompanionDiscussion,PostComments,isSourcePublicSquad) — the changed-file guard reports all errors in a touched file, not just new ones. No behaviour change, exceptDrawernow forwards the triggering event toonCloseinstead of passingundefined.Review follow-ups (
79e3aea)Laptopbreakpoint (tablet rotation), which remounts the editor between the drawer and inline trees —CommentInputholds it and reseeds viainitialContent.requestOpenPostCommentwarns in dev when nothing is listening, since a new post-page surface that forgetsuseOpenPostCommentRequestwould otherwise fail silently; subscription is centralized insubscribeOpenPostComment.useComposerSubmit(unrelated to comments) — it lands in feat(composer): edit shared posts and share-to-squad in the new composer #6422, where it has a spec.Events
No new tracking events.
Experiment
No new experiments.
Manual Testing
Measured on the preview rather than eyeballed, at 375px and 1280px:
0 → 812, editor focused on open, submit 32px above the bottom, one 20px frame on all four sides.End/PageDownmove the page 0px; the same keys after closing scroll it 150 → 550. Composer content still scrolls internally (562px of travel with 40 lines) and the submit stays on screen.Review round four (notification opt-in, dialog semantics, context bridge)
CommentModalcarried (NotificationPromptSource.NewComment): the switch stacks above the bottom action bar andonSubmittedfires only after a successful comment.BaseDrawernow behaves like the dialog it replaced:role="dialog"/aria-modal, initial focus, a Tab trap, Escape closes the top-most drawer of a stack, and focus returns to the opener on close.CustomEventbridge is gone.ActivePostContextProvider— which already wraps both the floating bar and the post content on the only page that used it — carriesrequestOpenComment/onOpenCommentRequest, deletinglib/postComment.ts, the listener count, and the dev-only warning. The PostPage test now clicks the real floating-bar button end to end.NewCommentbut never render underFooterNavBarLayoutwith a post (EngagementRail,ToolDiscussion) deliberately do not register the hook; the floating bar never targets them.forceInline): below the Laptop breakpoint the drawer would have scroll-locked the host page's<html>and covered the article.autoagain, so consumerrows(GiveAwardModal, OpportunityEdit, share commentary at 5–6 rows) floor the empty state as before; the comment composer keeps itsmin-h-[6rem].On those affected packages:
CompanionDiscussionupdated, tests pass, not exercised by handDid you test the modified components media queries?
Did you test on actual mobile devices?
Important
Still needs a real-device pass before merge. The keyboard path is verified by measurement in emulation only, and
visualViewport.offsetTop— which positions the drawer while the keyboard is open — is exactly the API that behaves differently on real iOS Safari. Theenv(safe-area-inset-bottom)branch of the action bar's padding also only activates on hardware.🤖 Generated with Claude Code
Preview domain
https://claude-mobile-comment-input-fix.preview.app.daily.dev